Make the boilerplate resolve the packages it claims, and rename in one command - #13
Conversation
magic_starter alpha.26 needs magic_notifications ^0.2.0, and this app pinned ^0.0.2, which means >=0.0.2 <0.1.0. The two do not intersect, so pub quietly walked the starter back to alpha.24 instead of failing: a fork got a build that resolved, ran, and was two releases stale. The two constraints move together or not at all. alpha.24 also introduced 20 magic_starter.titles.* keys that en.json never gained, and route_service_provider.dart:29 registers the notification routes ungated, so those titles have been rendering as raw keys. The two generated files come along because they are functions of the resolved graph rather than edits: flutter pub get performs a one-time SDK migration on analysis_options.yaml, and the macOS registrant gains purchases_flutter now that magic_payments arrives through the starter. The registrant records the LOCAL graph, which is what keeps the working tree clean. Unlike pubspec.lock, no fork ever consumes this file: any build regenerates it from whatever that fork resolves, so the committed value is transient for them and permanent dirt here would be the only lasting cost.
config/app.dart registers LocalizationServiceProvider and NotificationServiceProvider, but neither had a config file and neither appeared in main.dart's factory list, so both fell through to package defaults. The notifications feature flag was off besides, which meant a dependency this app pays for shipped undemonstrated. supported_locales already claimed en and tr while assets/lang held only en.json. That claim is now true. The parity between the two files is exact rather than best effort because magic's Translator replaces its sentence map on load instead of merging it with the fallback: a key missing from tr.json renders the raw key path on screen, not the English string. locale_parity_test.dart is what keeps that from drifting back. auto_detect_locale stays off. A device set to a locale this boilerplate does not ship has no tested path here, and the replace-not-merge behaviour above is exactly what turns that into raw keys on every screen. A fork flips it once its supported_locales covers the locales its users actually have.
Four independent things, all of which were wrong or missing today. CI gains a published job. Resolving is not the check: pub exits 0 on a downgrade as long as SOME version in range resolves, which is exactly how the alpha.24 walk-back stayed green. The job reads the resolved version out of the lock and asserts it, verified against a synthetic alpha.24 lock to confirm it fails on the input it exists to catch. The component-visual-reviewer agent pointed at /Users/anilcan/Code/fluttersdk/ lib/ui/components, missing this repo's own directory segment. That path does not exist, so every find and grep in its procedure returned nothing and reported a clean review. It was also the only absolute path in a repository whose whole purpose is to be cloned somewhere else. design.md described components as living in the magic_starter generic library on one line and scoped the WindRecipe import rule to magic_starter files on another. Both meant this app's own lib/ui/components/. The import rule itself was right and is unchanged. design-tokens rescanned raw on one trigger, an unterminated block comment at end of file. That catches the OPEN case, which flutter analyze already makes impossible in real Dart. The CLOSED case is the live one: a /* inside a triple-quoted string that some later ordinary */ closes leaves the scanner balanced at EOF, so nothing fires and the region between them is dropped in silence. Measured against the pre-edit script, a raw literal placed that way passed clean.
The lock was gitignored because one generated with pubspec_overrides.yaml active encodes sibling paths that exist on one machine. That diagnosis was right and the conclusion was not: the fix for a lock generated the wrong way is to generate it the right way. Untracked, a fork's first build resolves whatever the caret ranges happen to solve to on the day it clones, which is not what CI proved green. run_lockfile is what makes tracking safe. It reads the STAGED lock, not the working tree, and fails on a source: path entry. That split is deliberate and permanent: a local pub get puts sibling paths back in the lock every time, so the working copy stays dirty and unstaged while the index holds the hosted resolution. require_local_siblings reads the opposite state (the overrides file must be PRESENT), and the two cannot wedge because regenerating hosted-only moves that file aside and back rather than removing it. Verified by staging the real local-path lock, 11 path entries, and confirming the job fails with every other job green. An earlier probe that appended a malformed line proved nothing: pub refused to parse it and the run aborted before the gate executed.
app:rename rewrites 24 sites plus the Kotlin package directory: the Dart package name and its import sites, the Android namespace and applicationId, the iOS and macOS bundle identifiers, the three desktop build systems, the web manifest and title, .env, lib/main.dart, DESIGN.md and dependabot.yml. It derives the CURRENT identity from the tree rather than hardcoding it, which is what makes a second run a no-op. --dry-run is proven inert by hashing every file in scope before and after, not by reading git status, which cannot tell a sibling's staged file from a write. Rewrite rules scan for their anchor instead of visiting recorded line numbers. That is why the command handles the sixth PRODUCT_BUNDLE_IDENTIFIER line in the iOS project file, which the research inventory had missed. Two things the repo documented and never demonstrated now have exactly one instance each: a controller implementing SessionScopedController, so the SessionScopeSync.attach() call that has always run finally has something to reset, and a status token family in the supplement shape DESIGN.md describes. The tracked macOS plugin registrant switches to the hosted variant. A fork clones it, runs pub get against the published graph, and gets no diff, so app:rename does not refuse on its first run. The local variant is what made it refuse. The tracked lock already follows this rule for the same reason: what is committed is the graph a fork resolves, not the one this machine does.
…aths The status tokens asserted WCAG AA in their comments and missed it on four of six pairs, measured: white on #16A34A is 3.30, near-black on #B45309 is 3.53, white on #0EA5E9 is 2.77, white on #0284C7 is 4.10. The cause was assuming one foreground per role works in both modes, but the generated fills do not hold lightness across modes and they move in opposite directions: bg-success goes darker to lighter, bg-warning lighter to darker. So each foreground flips by mode now, no generated fill changed, and every pair is recomputed in the docblock: 5.93, 6.40, 5.02, 5.38, 5.57, 5.02. Two paths could act without the operator seeing it. The identity read back out of the tree was never validated, only the flags were, so a pubspec.yaml carrying a traversal in its name reached a directory move; it is now held to the same patterns. And the Kotlin package move skipped every non-File entity and then deleted the source recursively, which would take a fork's subpackages with it, silently, in a case this repo has no fixture for. --display now rejects # and tab. They break nothing syntactically, they get eaten: flutter_dotenv strips a trailing #comment, so --display='Acme #1' left the app reading 'Acme' while the command's own re-read saw the whole string, which quietly breaks idempotency. TEST_HOST on macOS is rewritten after all. The constraint was to touch the Xcode project only through anchored matches, and this rule anchors on the setting name and the current value exactly like the bundle-id rule beside it, so it is an instance of that constraint rather than an exception to it. Left alone it pointed a renamed fork's RunnerTests target at a bundle that is no longer produced. The lockfile gate did not run in CI at all, because CI never invokes bin/check; it now checks the committed lock directly, before pub get rewrites the evidence. The dashboard card was duplicated between its loading and loaded branches, differing only in indentation, and its two strings were hardcoded English on the one screen the rules name as the pattern to copy.
Nothing in this app imports file_picker. It arrives through magic's Pick facade, which makes the pin here a second opinion on a version this app has no stake in, and second opinions on a transitive dependency only matter when they disagree with the first one. They did. magic's master moved to ^12.2.0 ahead of a release while this app pinned ^11.0.2 to match published magic 0.0.9. Those ranges do not intersect, so version solving failed outright against local sibling checkouts and bin/check could not run at all, which is the state this repository was already in before any of this work started. Removing the pin resolves file_picker from whichever magic is in play: transitive 12.2.0 against the path-bound local checkout, transitive 11.0.3 against published magic 0.0.9, which is the same version a fork was getting before. Both graphs verified green with the full suite, and the day magic publishes its file_picker 12 port nothing here needs editing. The comment the pin carried argued a fork has no dependency_overrides safety net so the constraint should be repeated here. That was true and still insufficient: magic 0.0.9 already caps file_picker below 12.0.0-0, so the pin never protected anything the framework was not already protecting.
|
Note Kodizm (AI-generated). May contain mistakes; verify before acting. The dependency fix and its CI proof hold up under hosted resolution, and the localisation work checks out exactly as claimed; one latent correctness bug in Major
Minor
TestsGood coverage for the new surface: 16 cases for Checks I ran
|
_buildPlan spread the identity rewrites and the Dart-import scan into one list without merging by path, and the loop re-reads each path from disk. So a file claimed by both generators produced two planned writes from the same original source, and _apply performed them in order: the second discarded the first while the report counted the file as changed twice, with plausible line counts. lib/main.dart is the only path both can claim. It carries the MagicApplication title AND is scanned for a `package:` self-import. This repository never reaches the collision because its own main.dart imports relatively, which is exactly why the fix has to be structural rather than left to a fixture that mirrors this tree: a fork that self-imports loses its display name silently. The test introduces the self-import to reach the case, and fails on the old code with "the display-name rewrite was discarded by the import rewrite". The published job now reads the expected version floor out of pubspec.yaml instead of carrying a literal. A hard-coded alpha.26 would turn the job red on the next routine bump, in a file the bumping author has no reason to open, and name a version nobody typed. The comparison is a floor rather than an equality so a legitimately newer release still passes, verified against alpha.24 (rejected), alpha.26, alpha.27 and 0.0.2.
7bd5af2 to
257db66
Compare
|
Thanks, the Major is real and now fixed. Two notes where I landed somewhere different. Major, The plan now merges rules per path before planning. The test introduces the Minor, the hard-coded Minor, So this is a property of the artisan generators rather than of this change, and it predates the On |
|
Note Kodizm (AI-generated). May contain mistakes; verify before acting. Incremental re-review of 257db66 only: the duplicate-path Major is genuinely fixed at the structural level and the CI floor is now derived rather than typed; one residual edge in the Since my last review: Minor
TestsThe new case is the right one: it writes the Checks I ran
|
The nine constraints were written against what was published when this branch opened, and five releases have gone out since: magic 0.0.11, magic_deeplink 0.1.0, magic_notifications 0.3.1, magic_starter 0.0.27 and fluttersdk_artisan 0.0.15. Two of them were not lagging but EXCLUDING. A caret raises the minor whenever the major is zero, so magic_deeplink ^0.0.3 ends at 0.1.0 and magic_notifications ^0.2.0 ends at 0.3.0: the two releases that carry the cold-start push fix this app is the demonstration of could not be resolved here at all. Pub does not report that as a conflict, it walks another package backwards and exits zero, which is the failure this branch exists to close. The macOS plugin registrant moves with them: magic 0.0.10 took file_picker to 12, which splits the plugin into federated platform packages, so the generated import is file_picker_darwin rather than file_picker. It is a tracked generated file and has to match the graph the lock describes. Resolved hosted with no overrides, which is what the published CI job asserts. dart analyze clean, 27 tests green.
|
Note Kodizm (AI-generated). May contain mistakes; verify before acting. Incremental re-review of 4c3269c only (the dependency bump): the graph resolves cleanly and the lock is exact, but the Since my last review: nine pins moved ( Major
Minor
Tests
Checks I ran
|
app:rename refused a target whose Kotlin package would sit INSIDE the current one only by accident: the guard compared the two paths for equality, and --org=com.fluttersdk.magic_example --name=app names a real identity whose move is the package into a child of itself. The apply step creates the destination first and then lists the source, so the rename landed on the directory it had just made and threw EINVAL after the identity rewrites were already on disk, leaving a half-renamed tree with git as the only undo. It is refused at validation time now, and the message names the two-run route that reaches the same identity without nesting. Equality is excluded, because a display-only rename leaves the Kotlin path untouched. appleId camel-cased only the package, so an org carrying an underscore (com.acme_inc, the ordinary Java encoding of acme-inc.com) produced com.acme_inc.acmeApp in both pbxproj files, and CFBundleIdentifier takes alphanumerics, hyphen and period alone. Every segment is camel-cased now. The report prints androidId and appleId beside the three inputs, because the two ids no longer match character for character and a dry run that hid them made the difference a surprise at code-signing time. lib/config/notifications.dart declared no service worker keys. The web driver drops an absent key from the config it hands the SDK, which then registers OneSignalSDKWorker.js at the root scope that a Flutter web build already owns, so whichever registration lands second wins and push silently never arrives. The two literals match what notifications:install writes into web/, and a fork cannot recover them by re-running that command because it refuses to overwrite an existing config file. database.enabled stays and gains a comment rather than being removed: nothing at runtime gates on it, but notifications:configure prints and rewrites it, and the package's own install stub declares it. Also fixes the floor gate in ci.yml, which compared with sort -V. GNU filevercmp orders an empty suffix below -alpha.1, so a walk-back from 0.0.27 to 0.0.27-alpha.1 sorted the floor first and PASSED the assertion that exists to catch exactly that, and an alpha floor against a stable resolution failed the opposite way. The comparison is semver now, verified against both directions.
|
Note Kodizm (AI-generated). May contain mistakes; verify before acting. Incremental re-review of 653d31e only: all four fixes land correctly - the semver gate now behaves in both directions, the Kotlin nesting refusal fires before any write, Since my last review: Minor
TestsBoth new cases are the right ones and both fail for the stated reason if the guard is removed: the nesting test pins a full-tree snapshot to prove the refusal precedes any write, and the underscored-org test asserts Still open from my last reviewThe Checks I ran
|
1 participant
What was wrong
A fork of this repository was already resolving the wrong packages, silently.
pubspec.yamlpinnedmagic_notifications: ^0.0.2, which in Dart's caret semantics means>=0.0.2 <0.1.0.magic_starteralpha.26 requires^0.2.0. The two ranges do not intersect,so pub did not fail: it walked the starter back to
alpha.24and exited 0. Every fork got abuild that resolved, ran, and was two releases stale, and nothing in CI could see it.
Most of the rest follows from looking closely at that:
alpha.24introduced 20magic_starter.titles.*translation keys this app never shipped,while
route_service_provider.dart:29registers the routes that need them unconditionally.Those titles have been rendering as raw key paths.
LocalizationServiceProviderandNotificationServiceProviderwere both registered with noconfig file and no factory in
main.dart.supported_localesclaimed['en', 'tr']whileassets/lang/held onlyen.json..claude/agents/component-visual-reviewer.mdpointed three commands at<workspace>/lib/ui/components, missing this repo's own directory segment. That path doesnot exist, so every
findandgrepin its procedure returned nothing and reported a cleanreview.
What this changes
The graph a fork gets, and proof of it. Nine hosted constraints bumped, with
magic_starterandmagic_notificationsmoved as one edit because a partial bump does notsolve. A new
publishedCI job resolves hosted-only and asserts the resolved version out ofthe lock, because
pub getexits 0 on a downgrade and only the lock knows which version won.pubspec.lockis now tracked as the hosted resolution, gated byrun_lockfileinbin/checkplus a CI check that runs before
pub getcan rewrite the evidence.app:rename. The fork checklist was nine manual steps that never mentioned macOS, Windows,Linux, web,
Info.plist,main.dart,dependabot.yml, or the Kotlin package directory move.One command now rewrites 24 sites plus that directory move, derives the current identity from
the tree so a second run is a no-op, and refuses on a dirty worktree only when it is actually
writing.
--dry-runis proven inert by hashing all 158 files in scope before and after.Two documented patterns that had no instance. A controller implementing
SessionScopedController, so theSessionScopeSync.attach()call that has always run finallyhas something to reset, and a status token family in the supplement shape
DESIGN.mddescribes.
file_pickeris no longer pinned here. Nothing in this app imports it; it arrives throughmagic's
Pickfacade. The pin was a second opinion on a transitive dependency, and whenmagic's master moved to
^12.2.0ahead of a release the two ranges stopped intersecting, solocal resolution failed outright and
bin/checkcould not run at all. It now resolves fromwhichever magic is in play: 12.2.0 locally, 11.0.3 hosted, which is what a fork was already
getting.
Verification
bin/checkgreen, eight jobs.CHECK_ALLOW_HOSTED=1 bin/checkgreen too, so the suite passesagainst the packages a fork actually downloads and not just the local checkouts.
app:renamewas run for real in a throwaway worktree:
pub getleft it clean, the rename rewrote 24 filesand moved the Kotlin package,
flutter analyzecame back clean against the hosted graph, and asecond identical run reported
unchanged (24)with an empty diff. Tests went from 10 to 26.Two gates were negative-controlled rather than assumed: the lockfile gate fires on a genuinely
path-poisoned lock with every other job green, and the hardened
design-tokensscanner catchesa raw colour literal that the previous version passed clean.
Not verified
The dusk-driven pass at 1440px and 390px did not run.
bin/fsadeadlocks on its own lock: itacquires it, installs a
trap ... EXITto release it, thenexecs into the compiled binary atbin/fsa:110, andexecreplaces the shell so the trap never fires. Short commands self-healwhen the process exits; the long-running
mcp:servethis project spawns never releases. Thedispatcher route works but repoints a shared
~/.artisan/state.jsonthat other live sessionswere holding.
Static cover was built instead: every
magic_starter.titles.*key the starter's sourcereferences was compared against what this app ships, an exact 20-to-20 match, and a new
locale_parity_test.dartholdsenandtrto identical key sets. That covers the cause of araw route title, not a widget-level key or a layout break at 390px.
The
bin/fsalock bug is not fixed here, since no part of this change owns that script, butevery fork inherits it.